Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test(terraform): run tests for multiple tf versions #4381

Merged
merged 24 commits into from
Jun 1, 2023
Merged

test(terraform): run tests for multiple tf versions #4381

merged 24 commits into from
Jun 1, 2023

Conversation

stefreak
Copy link
Member

@stefreak stefreak commented May 22, 2023 •
edited by Walther
Loading

What this PR does / why we need it:

  • test(terraform): run tests for multiple tf versions
  • improvement: remove terraform lock files from git tree

Which issue(s) this PR fixes:

Fixes #4507

Special notes for your reviewer:

See comment thread below - this turned out to have a bit of complexity in it and spawned some new follow-up issues.

@stefreak stefreak changed the title test terraform latest test(terraform): run tests for multiple tf versions May 22, 2023
This was referenced May 22, 2023
Closed
Closed
@Walther Walther force-pushed the test-terraform-latest branch from 948b096 to 08f0ef2 Compare May 23, 2023 11:23
@stefreak stefreak mentioned this pull request May 26, 2023
Closed
@Walther Walther mentioned this pull request May 26, 2023
Closed
@Walther Walther marked this pull request as ready for review May 26, 2023 16:05
@Walther
Copy link
Contributor

Walther commented May 26, 2023 •
edited
Loading

All of the terraform-plugin tests pass now locally 🎉 Will look into the CI issue soon.

We should probably do another round of refactoring to clean these tests up (possibly in another PR). They have accumulated incremental changes over time and there's quite a lot of clashing between global and local variables. That was a major contributor to why fixing the tests (and understanding them in their broken state) took such a long time.

Additionally, work on this PR uncovered this issue in our log implementation #4457

@Walther
Copy link
Contributor

Walther commented May 30, 2023

Blocked by #4467

This was referenced May 30, 2023
Closed
Closed
This was referenced May 30, 2023
Closed
Closed
Walther added 2 commits May 31, 2023 17:23
@Walther
refactor: improve log message methods for testing purposes
a2b8371 
based on the feedback provided at #4457 (comment)
@Walther
chore: return the separate getRootLogMessages and getLogMessages
4334ce9 
other unrelated tests were depending on the specific behavior.
we can always come back to refactor further.
@Walther Walther force-pushed the test-terraform-latest branch from 24446c1 to 4334ce9 Compare May 31, 2023 15:59
@Walther
Copy link
Contributor

Walther commented May 31, 2023 •
edited
Loading

All of the terraform tests pass locally

 70 passing (4m)

✨  Done in 216.82s.
yarn test  113.29s user 32.90s system 67% cpu 3:37.04 total

The test-plugins step fails in CI though due to #4467
Additional discussion on that issue on Discord here.

I suggest we review and merge this PR as-is, to unblock other work (including other plugin test fixes which depend on this PR) and fix the corruption bug separately. This will sadly cause some redness in our CI, but should be temporary.

@Walther Walther requested a review from a team May 31, 2023 16:09
@Orzelius Orzelius mentioned this pull request Jun 1, 2023
Merged
@Walther
chore: disable terraform-plugin tests
7b8aa63 
The tests have been fixed and they pass locally. These tests fail in CI
because of #4467
Temporarily skipping these tests in order to unblock other PRs for the
other plugins and fixing their test setups, while keeping our CI green
for the test-plugins step.
@Walther Walther mentioned this pull request Jun 1, 2023
Closed
@Walther
Copy link
Contributor

Walther commented Jun 1, 2023 •
edited
Loading

I have now added .skip() calls to all of the terraform tests due to the executable corruption bug, to make the CI green for test-plugins.
Merging this PR fixes #4507 and unblocks some other PRs, including fixing and re-enabling tests for the other plugins.
We have a separate tracking issue for re-enabling these tests here #4473, to be completed after fixing #4467

Orzelius
Orzelius previously approved these changes Jun 1, 2023
View reviewed changes
Copy link
Contributor

@Orzelius Orzelius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some small comments and questions

plugins/terraform/test/terraform.ts Show resolved Hide resolved
plugins/terraform/test/test-project-action/tf/.terraform.lock.hcl Outdated Show resolved Hide resolved
plugins/terraform/test/test-project-module/garden.yml Outdated Show resolved Hide resolved
@Walther Walther mentioned this pull request Jun 1, 2023
Closed
@Walther Walther requested a review from Orzelius June 1, 2023 13:54
@Walther Walther requested a review from a team June 1, 2023 14:10
vvagaytsev
vvagaytsev approved these changes Jun 1, 2023
View reviewed changes
Copy link
Collaborator

@vvagaytsev vvagaytsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🎉

@Walther Walther merged commit 532545e into main Jun 1, 2023
@Walther Walther deleted the test-terraform-latest branch June 1, 2023 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Walther Walther Walther left review comments

@vvagaytsev vvagaytsev vvagaytsev approved these changes

@Orzelius Orzelius Awaiting requested review from Orzelius

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

chore: fix terraform-plugin tests
4 participants
@stefreak @Walther @vvagaytsev @Orzelius